[lldb][headers] Create script to fix up versioning #141116
Conversation
|
@llvm/pr-subscribers-lldb Author: Chelsea Cassanova (chelcassanova) ChangesThis commit creates a Python script that fixes up the versioning information in lldb-defines.h. It also moves the build logic for fixing up the lldb headers from being in the framework only to being in the same location that we create the liblldb target. Full diff: https://github.com/llvm/llvm-project/pull/141116.diff 4 Files Affected:
diff --git a/lldb/scripts/version-header-fix.py b/lldb/scripts/version-header-fix.py
new file mode 100755
index 0000000000000..ddf2c9057fdc3
--- /dev/null
+++ b/lldb/scripts/version-header-fix.py
@@ -0,0 +1,53 @@
+#!/usr/bin/env python3
+"""
+Usage: <path/to/input-header.h> <path/to/output-header.h> LLDB_MAJOR_VERSION LLDB_MINOR_VERSION LLDB_PATCH_VERSION
+
+This script uncomments and populates the versioning information in lldb-defines.h
+"""
+
+import argparse
+import os
+import re
+
+LLDB_VERSION_REGEX = re.compile(r'^//#define LLDB_VERSION$')
+LLDB_REVISION_REGEX = re.compile(r'^//#define LLDB_REVISION$')
+LLDB_VERSION_STRING_REGEX = re.compile(r'^//#define LLDB_VERSION_STRING$')
+
+def main():
+ parser = argparse.ArgumentParser()
+ parser.add_argument("input_path")
+ parser.add_argument("output_path")
+ parser.add_argument("lldb_version_major")
+ parser.add_argument("lldb_version_minor")
+ parser.add_argument("lldb_version_patch")
+ args = parser.parse_args()
+ input_path = str(args.input_path)
+ output_path = str(args.output_path)
+ lldb_version_major = args.lldb_version_major
+ lldb_version_minor = args.lldb_version_minor
+ lldb_version_patch = args.lldb_version_patch
+
+ with open(input_path, "r") as input_file:
+ lines = input_file.readlines()
+
+ with open(output_path, "w") as output_file:
+ for line in lines:
+ version_match = LLDB_VERSION_REGEX.match(line)
+ revision_match = LLDB_REVISION_REGEX.match(line)
+ version_string_match = LLDB_VERSION_STRING_REGEX.match(line)
+
+ # For the defines in lldb-defines.h that define the major, minor and version string
+ # uncomment each define and populate its value using the arguments passed in.
+ # e.g. //#define LLDB_VERSION -> #define LLDB_VERSION <LLDB_MAJOR_VERSION>
+ if version_match:
+ output_file.write(re.sub(LLDB_VERSION_REGEX, r'#define LLDB_VERSION ' + lldb_version_major, line))
+ elif revision_match:
+ output_file.write(re.sub(LLDB_REVISION_REGEX, r'#define LLDB_REVISION ' + lldb_version_minor, line))
+ elif version_string_match:
+ output_file.write(re.sub(LLDB_VERSION_STRING_REGEX, r'#define LLDB_VERSION_STRING "{0}.{1}.{2}"'.format(lldb_version_major, lldb_version_minor, lldb_version_patch), line))
+ else:
+ output_file.write(line)
+
+
+if __name__ == "__main__":
+ main()
diff --git a/lldb/source/API/CMakeLists.txt b/lldb/source/API/CMakeLists.txt
index 3bc569608e458..a1f2d4fee82c8 100644
--- a/lldb/source/API/CMakeLists.txt
+++ b/lldb/source/API/CMakeLists.txt
@@ -290,6 +290,40 @@ else()
endif()
endif()
+# Stage all headers in the include directory in the build dir.
+file(GLOB public_headers ${LLDB_SOURCE_DIR}/include/lldb/API/*.h)
+set(lldb_header_staging ${CMAKE_BINARY_DIR}/include/lldb)
+file(GLOB root_public_headers ${LLDB_SOURCE_DIR}/include/lldb/lldb-*.h)
+file(GLOB root_private_headers ${LLDB_SOURCE_DIR}/include/lldb/lldb-private*.h)
+list(REMOVE_ITEM root_public_headers ${root_private_headers})
+foreach(header
+ ${public_headers}
+ ${generated_public_headers}
+ ${root_public_headers})
+ get_filename_component(basename ${header} NAME)
+ set(staged_header ${lldb_header_staging}/${basename})
+
+ if(unifdef_EXECUTABLE)
+ # unifdef returns 0 when the file is unchanged and 1 if something was changed.
+ # That means if we successfully remove SWIG code, the build system believes
+ # that the command has failed and stops. This is undesirable.
+ set(copy_command ${unifdef_EXECUTABLE} -USWIG -o ${staged_header} ${header} || (exit 0))
+ else()
+ set(copy_command ${CMAKE_COMMAND} -E copy ${header} ${staged_header})
+ endif()
+
+ add_custom_command(
+ DEPENDS ${header} OUTPUT ${staged_header}
+ COMMAND ${copy_command}
+ COMMENT "LLDB headers: stage LLDB headers in include directory")
+
+ list(APPEND lldb_staged_headers ${staged_header})
+endforeach()
+
+add_custom_command(TARGET liblldb POST_BUILD
+ COMMAND ${LLDB_SOURCE_DIR}/scripts/version-header-fix.py ${LLDB_SOURCE_DIR}/include/lldb/lldb-defines.h ${lldb_header_staging}/lldb-defines.h ${LLDB_VERSION_MAJOR} ${LLDB_VERSION_MINOR} ${LLDB_VERSION_PATCH}
+)
+
if(LLDB_BUILD_FRAMEWORK)
include(LLDBFramework)
diff --git a/lldb/test/Shell/Scripts/Inputs/lldb-defines.h b/lldb/test/Shell/Scripts/Inputs/lldb-defines.h
new file mode 100644
index 0000000000000..eed1b57d35ad3
--- /dev/null
+++ b/lldb/test/Shell/Scripts/Inputs/lldb-defines.h
@@ -0,0 +1,16 @@
+//===-- lldb-defines.h ------------------------------------------*- C++ -*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+// LLDB version
+//
+// This is a truncated version of lldb-defines.h used to test the script
+// that fixes up its versioning info.
+
+// The script needs to uncomment these lines and populate the info for versioning.
+//#define LLDB_VERSION
+//#define LLDB_REVISION
+//#define LLDB_VERSION_STRING
diff --git a/lldb/test/Shell/Scripts/TestVersionFixScript.test b/lldb/test/Shell/Scripts/TestVersionFixScript.test
new file mode 100644
index 0000000000000..6acecd172f052
--- /dev/null
+++ b/lldb/test/Shell/Scripts/TestVersionFixScript.test
@@ -0,0 +1,13 @@
+// Run the convert script on it, then run the framework include fix on it. The framework version fix script
+// expects that all lldb references have been renamed to lldb-rpc in order for it to modify the includes
+// to go into the framework.
+# RUN: mkdir -p %t/Outputs
+# RUN: %python %p/../../../scripts/version-header-fix.py %p/Inputs/lldb-defines.h %t/Outputs/lldb-defines.h 21 0 0
+
+// Check the output
+# RUN: cat %t/Outputs/lldb-defines.h | FileCheck %s
+
+// The LLDB version defines must be uncommented and filled in with the values passed into the script.
+# CHECK: {{^}}#define LLDB_VERSION 21
+# CHECK: {{^}}#define LLDB_REVISION 0
+# CHECK: {{^}}#define LLDB_VERSION_STRING "21.0.0"
|
chelcassanova
force-pushed
the
create-python-version-fix-script
branch
from
8e33443 to
6b43888
Compare
|
✅ With the latest revision this PR passed the Python code formatter. |
chelcassanova
force-pushed
the
create-python-version-fix-script
branch
from
6b43888 to
70c3d43
Compare
bulbazord
left a comment
bulbazord
left a comment
There was a problem hiding this comment.
There is functionality in lldb/scripts/framework-header-fix.sh that does the same thing. Can you remove this functionality from that script too?
Yeah, I can remove that. The ultimate goal I want here is:
|
lldb/scripts/version-header-fix.py
Outdated
There was a problem hiding this comment.
Not that performance really matters here, but any reason to not do the substring replace on the whole buffer and make the regexes slightly less pedantic (e.g. now this script will fail if you have a space before or after the strings we're matching. Unless that's intentional?
There was a problem hiding this comment.
Unless that's intentional?
Not really, this is more of a holdover from how I set up the regex matching for other Python scripts that do this kind of thing. I can try passing in the buffer here instead of going line by line.
There was a problem hiding this comment.
We don't need the header in tests.
| //===-- lldb-defines.h ------------------------------------------*- C++ -*-===// | |
| // | |
| // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions. | |
| // See https://llvm.org/LICENSE.txt for license information. | |
| // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception | |
| // | |
| //===----------------------------------------------------------------------===// |
There was a problem hiding this comment.
- What's "it"?
- The comment seems wrong as it's talking about lldb-rpc
There was a problem hiding this comment.
This can be removed, this comment is referring to lldb-rpc and the "it" would've been from when I was copying a file in from source.
chelcassanova
force-pushed
the
create-python-version-fix-script
branch
5 times, most recently
from
23a443b to
81ec17a
Compare
This commit creates a Python script that fixes up the versioning information in lldb-defines.h. It also moves the build logic for fixing up the lldb headers from being in the framework only to being in the same location that we create the liblldb target.
chelcassanova
force-pushed
the
create-python-version-fix-script
branch
from
81ec17a to
40793cd
Compare
| endif() | ||
| endif() | ||
|
|
||
| # Stage all headers in the include directory in the build dir. |
There was a problem hiding this comment.
This looks very similar to some logic in LLDBFramework.cmake. Do we need this logic in both places or could it only live here?
There was a problem hiding this comment.
The logic of finding all the headers for staging them in a directory should be able to live here alone. The idea is that the framework would then use the staged headers in <build-dir>include/lldb as its own input for when headers have to be fixed up for a framework build (see LLDBFramework.cmake in this patch: https://github.com/llvm/llvm-project/pull/142051/files where the logic for finding headers is removed and instead uses the staged headers from the build dir)
| # unifdef returns 0 when the file is unchanged and 1 if something was changed. | ||
| # That means if we successfully remove SWIG code, the build system believes | ||
| # that the command has failed and stops. This is undesirable. | ||
| set(copy_command ${unifdef_EXECUTABLE} -USWIG -o ${staged_header} ${header} || (exit 0)) |
There was a problem hiding this comment.
@chelcassanova
For the unifdef executable, -o option is not available in some of the system like AIX, and its giving warning while building lldb on AIX.
Is there any way to generate the output file without -o? may be using '>' operation ?
[362/3610] LLDB headers: stage LLDB headers in include directory
/usr/bin/unifdef: 1286-201 -o is not a recognized flag.
Usage: /usr/bin/unifdef [-t] [-l] [-c]
[[-DSymbol] [-USymbol] [-idSymbol] [-iuSymbol]] ... [File]
At least one parameter from [-D -U -id -iu] is required.
There was a problem hiding this comment.
This looks like it works (for me, on a local build at least). I updated this line to use > for output in the patch that relands this.
|
This also appears to be breaking the Xcode build: |
|
Can you please revert this while we investigate both issues? |
This reverts commit d204aa9.
Reverts #141116. It's breaking the Xcode build as well as the build on AIX.
Just reverted in #142864 |
I think this should be possible, although trying this locally caused the output file to be completely blank. I'll try doing this in the build system and see if it works there.
So To resolve this build failure, I think the target dependency in |
…" (#142864) Reverts llvm/llvm-project#141116. It's breaking the Xcode build as well as the build on AIX.
|
Patch to reland this patch: #142871 |
#142871) This relands the original commit for the versioning script in LLDB. This commit uses '>' for output from `unifdef` for platforms that have that executable but do not have the `-o` option. It also fixes the Xcode build by adding a dependency between the liblldb-header-staging target in the source/API/CMakeLists.txt the `liblldb-resource-headers` target in LLDBFramework.cmake. Original patch: #141116
…" (#142864)" (#142871) This relands the original commit for the versioning script in LLDB. This commit uses '>' for output from `unifdef` for platforms that have that executable but do not have the `-o` option. It also fixes the Xcode build by adding a dependency between the liblldb-header-staging target in the source/API/CMakeLists.txt the `liblldb-resource-headers` target in LLDBFramework.cmake. Original patch: llvm/llvm-project#141116
)" (llvm#142871) This relands the original commit for the versioning script in LLDB. This commit uses '>' for output from `unifdef` for platforms that have that executable but do not have the `-o` option. It also fixes the Xcode build by adding a dependency between the liblldb-header-staging target in the source/API/CMakeLists.txt the `liblldb-resource-headers` target in LLDBFramework.cmake. Original patch: llvm#141116
)" (llvm#142871) This relands the original commit for the versioning script in LLDB. This commit uses '>' for output from `unifdef` for platforms that have that executable but do not have the `-o` option. It also fixes the Xcode build by adding a dependency between the liblldb-header-staging target in the source/API/CMakeLists.txt the `liblldb-resource-headers` target in LLDBFramework.cmake. Original patch: llvm#141116 (cherry picked from commit eb76d83)
On Windows, the post build command would open the script in the default editor, since it doesn't know about shebangs. This effectively adds `python3` in front of the command. Amends llvm/llvm-project#142871 / llvm/llvm-project#141116
5 participants
This commit creates a Python script that fixes up the versioning information in lldb-defines.h. It also moves the build logic for fixing up the lldb headers from being in the framework only to being in the same location that we create the liblldb target.